-
-
Notifications
You must be signed in to change notification settings - Fork 810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: let params of internal functions be mutable #3473
feat: let params of internal functions be mutable #3473
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #3473 +/- ##
==========================================
+ Coverage 89.30% 89.35% +0.05%
==========================================
Files 84 84
Lines 10792 10777 -15
Branches 2461 2456 -5
==========================================
- Hits 9638 9630 -8
+ Misses 756 751 -5
+ Partials 398 396 -2
... and 9 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
please add some tests with complex types: static arrays, dynarrays and structs. |
vyper/semantics/analysis/local.py
Outdated
# allow internal function params to be mutable | ||
location = DataLocation.MEMORY if self.func.is_internal else DataLocation.CALLDATA | ||
is_immutable = False if self.func.is_internal else True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't really like repeating branches in general. here i think we can get rid of it with either squashing all into a single ternary like so
# allow internal function params to be mutable | |
location = DataLocation.MEMORY if self.func.is_internal else DataLocation.CALLDATA | |
is_immutable = False if self.func.is_internal else True | |
# allow internal function params to be mutable | |
location, is_immutable = (DataLocation.MEMORY, False) if self.func.is_internal else (DataLocation.CALLDATA, True) |
or two assignments per branch
# allow internal function params to be mutable | |
location = DataLocation.MEMORY if self.func.is_internal else DataLocation.CALLDATA | |
is_immutable = False if self.func.is_internal else True | |
if self.func.is_internal: | |
location = DataLocation.MEMORY | |
is_immutable = False | |
else: | |
location = DataLocation.CALLDATA | |
is_immutable = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also add tests which check that struct fields / array members are mutable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - thanks!
### 🕓 Changelog Given that the function parameters of `internal` functions are mutable (as implemented [here](vyperlang/vyper#3473)), this PR refines the internal logic of the `_log2`, `_log10`, `_log256`, `_wad_ln`, and `_wad_exp` functions to align with this behaviour. Additionally, it removes the unnecessary `denominator` variable declaration in the `erc2981` contract and addresses minor formatting issues in code comments. --------- Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
What I did
Fix #3449.
How I did it
Update attributes in
VarInfo
andVariableRecord
.How to verify it
See tests.
Commit message
Description for the changelog
Let params of internal functions be mutable.
Cute Animal Picture